Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial pass of libctru version checking in ctru-sys #101

Merged
merged 5 commits into from
Apr 1, 2023

Conversation

ian-h-chamberlain
Copy link
Member

@ian-h-chamberlain ian-h-chamberlain commented Mar 17, 2023

Partially addresses #100, but we'd probably want to do something as part of the CI release process as well.

This isn't set in stone, I'm just experimenting with some different things to see what makes sense. So far this seems to work pretty well, but I'm not sure if we want to encode the "duplicate" info with the + metadata on the build version as well as with the major/minor scheme mentioned in #100

Let me know what you think, I'll leave as a draft for now and we can discuss it.

@ian-h-chamberlain ian-h-chamberlain requested a review from a team March 17, 2023 17:30
@ian-h-chamberlain ian-h-chamberlain self-assigned this Mar 17, 2023
@ian-h-chamberlain ian-h-chamberlain changed the title Initial pass of version checking in ctru-sys Initial pass of libctru version checking in ctru-sys Mar 17, 2023
ctru-sys/build.rs Outdated Show resolved Hide resolved
Also bump doxgyen-rs to crates.io version, and mark bindings.rs as
generated for Github.
Also simplify version checking logic a bit. It's just a warning, so we
can check for exact version just to be safe and skip the major/minor
comparisons for semver.

Also update the README a bit to explain the versioning scheme.
@ian-h-chamberlain ian-h-chamberlain marked this pull request as ready for review March 20, 2023 18:07
@Meziu
Copy link
Member

Meziu commented Mar 25, 2023

We should let people the have the choice to use an external libctru binary rather than requiring dkp-pacman. It would be helpful to ensure compatibility in the future.

@ian-h-chamberlain
Copy link
Member Author

We should let people the have the choice to use an external libctru binary rather than requiring dkp-pacman. It would be helpful to ensure compatibility in the future.

That's a fair point. This change doesn't prevent anything like that, it would just display a warning that we couldn't determine the libctru version (and even then, that would only be displayed if using -vv or the compilation/linking failed). Hopefully that warning would give the user a clue that they might need to upgrade or use a supported version instead.

Copy link
Member

@Meziu Meziu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@ian-h-chamberlain ian-h-chamberlain merged commit 855dc46 into master Apr 1, 2023
@ian-h-chamberlain ian-h-chamberlain deleted the feature/check-libctru-version branch April 1, 2023 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants